AVRO-4241: [Java] Bound zero-byte collection elements per datum, not per collection - #3927
Merged
RyanSkraba merged 2 commits intoAug 7, 2026
Conversation
…per collection The heap-aware zero-byte-element allocation cap (null, a zero-length fixed, an all-zero-byte record, or a recursive schema broken with a 0 minimum) was enforced per collection: readArray/readCollection and the skip/fast-reader paths each started counting from zero. Because a container file carries its own schema, an attacker can declare a record with many array<null> fields, each block individually under the limit but jointly unbounded, so a tiny payload still drives a huge aggregate allocation (e.g. ~16 array<null> fields near the per-array cap exhaust the heap; a handful burn tens of seconds of CPU). Track the cumulative zero-byte allocation per decode on a per-thread scope in SystemLimitException. GenericDatumReader.read and the static skip open the scope (scopes nest, so a delegated fast reader or a skipped writer field accumulates into the enclosing datum budget instead of resetting it); only the outermost scope resets the running total. All zero-byte call sites (GenericDatumReader read/skip, FastReaderBuilder, ReflectDatumReader) now use the cumulative checkMaxCollectionAllocation(long). Outside any scope the check falls back to the previous per-collection behaviour, so no existing caller becomes stricter. Positive-size elements are unchanged: they remain bounded per collection by the bytes-remaining check, which consumes input as it advances. Adds regression tests for a multi-field record rejected cumulatively and a within-limit record that still decodes (and confirms the budget resets between datums), on both the fast and classic reader paths.
There was a problem hiding this comment.
Pull request overview
This PR hardens the Java Avro decoder’s collection-allocation guards by making the “zero-byte element” allocation cap cumulative across an entire decoded datum (not reset per collection), closing an amplification vector where many array<null>-style fields could jointly drive large heap allocations from tiny inputs.
Changes:
- Added per-thread, per-datum allocation scoping in
SystemLimitExceptionand a new cumulativecheckMaxCollectionAllocation(long items)API. - Delimited allocation scopes around top-level
GenericDatumReader.read(...)andGenericDatumReader.skip(...), and updated classic/reflect/fast-reader call sites to use the cumulative check. - Added new unit tests verifying cumulative rejection across multiple fields and budget reset between datums for both classic and fast reader paths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java | Adds tests ensuring cumulative zero-byte allocation caps apply across fields and reset between datums. |
| lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java | Introduces per-thread nested allocation scopes and a cumulative allocation-check overload. |
| lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java | Switches zero-byte allocation checks to the new cumulative API during array decoding. |
| lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java | Updates fast array decoding to use cumulative allocation checks for zero-byte elements. |
| lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java | Opens/closes per-datum allocation scopes for reads and skips; updates zero-byte allocation call sites. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ive standalone Open a collection-allocation scope around the fast array reader's block-reading loop in a try/finally. When the fast reader is used standalone via createDatumReader(...), without GenericDatumReader.read opening the outer datum scope, the zero-byte element cap is now cumulative across all array blocks instead of degrading to a per-block stateless check, so a large array<null>-style array split across many blocks cannot bypass the cap. The scope nests into the outer datum scope on the normal path, and the finally guarantees it is always closed so ThreadLocal state cannot leak into later decodes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:491
- SystemLimitException.beginCollectionAllocationScope() is called unconditionally for every array decode, even when zeroByteElements is false (i.e., the allocation cap will never be consulted). This adds ThreadLocal + depth bookkeeping overhead to the fast reader hot-path for all arrays. Consider only opening the allocation scope when zeroByteElements is true; endCollectionAllocationScope() can remain in the finally (it’s a no-op when depth==0).
// always closed so ThreadLocal state cannot leak into later decodes on the
// same thread.
SystemLimitException.beginCollectionAllocationScope();
try {
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:544
- The comment above checkMaxCollectionAllocation(count) says the cap is cumulative across the enclosing datum scope so “a record of many small array-style fields cannot over-allocate in aggregate”. That guarantee only holds when an outer datum scope is active (e.g., via GenericDatumReader.read/skip); when this fast reader is used standalone, the scope opened here is per-array. Rewording the comment to describe accumulation across the active allocation scope would avoid overstating the guarantee.
// apply the heap-aware allocation cap instead. The cap is cumulative across
// the enclosing datum scope (see SystemLimitException), so a record of many
// small array<null>-style fields cannot over-allocate in aggregate.
RyanSkraba
approved these changes
Aug 7, 2026
Contributor
|
Cherry-picked to branch-1.12. |
RyanSkraba
pushed a commit
that referenced
this pull request
Aug 7, 2026
…per collection (#3927) * AVRO-4241: [Java] Bound zero-byte collection elements per datum, not per collection The heap-aware zero-byte-element allocation cap (null, a zero-length fixed, an all-zero-byte record, or a recursive schema broken with a 0 minimum) was enforced per collection: readArray/readCollection and the skip/fast-reader paths each started counting from zero. Because a container file carries its own schema, an attacker can declare a record with many array<null> fields, each block individually under the limit but jointly unbounded, so a tiny payload still drives a huge aggregate allocation (e.g. ~16 array<null> fields near the per-array cap exhaust the heap; a handful burn tens of seconds of CPU). Track the cumulative zero-byte allocation per decode on a per-thread scope in SystemLimitException. GenericDatumReader.read and the static skip open the scope (scopes nest, so a delegated fast reader or a skipped writer field accumulates into the enclosing datum budget instead of resetting it); only the outermost scope resets the running total. All zero-byte call sites (GenericDatumReader read/skip, FastReaderBuilder, ReflectDatumReader) now use the cumulative checkMaxCollectionAllocation(long). Outside any scope the check falls back to the previous per-collection behaviour, so no existing caller becomes stricter. Positive-size elements are unchanged: they remain bounded per collection by the bytes-remaining check, which consumes input as it advances. Adds regression tests for a multi-field record rejected cumulatively and a within-limit record that still decodes (and confirms the budget resets between datums), on both the fast and classic reader paths. * AVRO-4241: [Java] Scope fast array reader so zero-byte cap is cumulative standalone Open a collection-allocation scope around the fast array reader's block-reading loop in a try/finally. When the fast reader is used standalone via createDatumReader(...), without GenericDatumReader.read opening the outer datum scope, the zero-byte element cap is now cumulative across all array blocks instead of degrading to a per-block stateless check, so a large array<null>-style array split across many blocks cannot bypass the cap. The scope nests into the outer datum scope on the normal path, and the finally guarantees it is always closed so ThreadLocal state cannot leak into later decodes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Follow-up to the AVRO-4241 / AVRO-4300 collection-allocation guards. Those changes capped the number of zero-byte-minimum collection elements (
null, a zero-lengthfixed, an all-zero-byte record, or a recursive schema whose cycle is broken with a0minimum) that a decoder will allocate, since such elements consume no input and so cannot be bounded by the "bytes remaining" check.However, the cap was enforced per collection:
readArray/readCollection, the staticskip, and the fast-reader path each start counting from zero. Because an Avro container file carries its own schema, an attacker can declare a record with manyarray<null>fields, each block individually under the limit but jointly unbounded. A tiny payload therefore still drives a huge aggregate allocation (a record of ~16array<null>fields near the per-array cap exhausts the heap; a handful burns tens of seconds of CPU). This is the Java counterpart of the Python fix in #3926 (AVRO-4296).Approach
Track the cumulative zero-byte allocation per decode on a per-thread scope in
SystemLimitException:beginCollectionAllocationScope()/endCollectionAllocationScope()delimit a datum. Scopes nest via a depth counter: a delegated fast reader or a skipped writer field accumulates into the enclosing datum budget instead of resetting it; only the outermost scope resets the running total (and clears it on exit so nothing leaks to a later decode on the same thread).GenericDatumReader.read(D, Decoder)and the staticGenericDatumReader.skip(Schema, Decoder)open the scope in atry/finally.readcovers the classic and (delegated) fast paths as well asSpecificDatumReader/ReflectDatumReader, which inherit it;skipcovers schema-projection skips andBinaryData(each top-levelskipis bounded per invocation).checkMaxCollectionAllocation(long items)accumulates into the active scope. Outside any scope it falls back to the existing per-collection check, so no existing caller becomes stricter. The zero-byte call sites inGenericDatumReader(read/skip),FastReaderBuilder, andReflectDatumReadernow use it.Positive-size elements are unchanged: they remain bounded per collection by the bytes-remaining check, which consumes input as the position advances.
How was this patch tested?
recordOfNullArrayFieldsRejectedCumulativelyAcrossDatum(a record with twoarray<null>fields of 600 each is rejected on the second field at a 1000-element cap) andrecordOfNullArrayFieldsWithinCumulativeLimitStillDecodes(two 400-element fields still decode, and the budget resets between datums), both exercised on the fast and classic reader paths.TestGenericDatumReader,TestReflectDatumReader, andTestSystemLimitExceptionpass, plusTestBinaryData,TestDataFile*,TestGenericData,TestSpecificData, andTestResolvingIOas regression coverage for theskip/compare and datafile paths.